ssh: Remove nowarn_possibly_unsafe_function compile directives#11006
ssh: Remove nowarn_possibly_unsafe_function compile directives#11006u3s wants to merge 1 commit intoerlang:masterfrom
Conversation
Replace binary_to_atom/1 with binary_to_existing_atom/1 in ssh_transport (valid_key_sha_alg_ec/2, public_algo/1). The input comes from ssh_message:oid2ssh_curvename/1 which returns a fixed set of 5 binaries whose atoms already exist as literals in the same module. Replace list_to_atom/1 with an explicit pattern match in ssh_connection (pty_default_dimensions/2). The Dimension argument is always the hardcoded atom width or height. Add a justification comment for the kept file:consult/1 directive in ssh_options — the file path is operator-controlled, not from wire data.
CT Test Results 2 files 29 suites 27m 19s ⏱️ Results for commit 8fca863. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
| valid_key_sha_alg_ec(OID, Alg) when is_tuple(OID) -> | ||
| {SshCurveType, _} = ssh_message:oid2ssh_curvename(OID), | ||
| Alg == binary_to_atom(SshCurveType); | ||
| Alg == binary_to_existing_atom(SshCurveType); |
There was a problem hiding this comment.
At this point, can you be sure that all the existing/valid SshCurveType values have already been "atomized"? That is, can you be sure that the valid atoms exist?
There was a problem hiding this comment.
there are only 5 SshCurveType binaries which can be returned by ssh_message:oid2ssh_curvename/1. all corresponding atoms are defined in ssh_transport:supported_algorithms/1 and ssh_transport:sha/1, and loaded to atom table upon loading ssh_transport beam module.
There was a problem hiding this comment.
... but in that case, if you know all the binaries you can get from that function and also the atoms they map to, why is this done through binary_to_existing_atom, instead of having a function like ssh_curvename_to_atom that turns one into the other, either in this module or in ssh_message?
3 participants
Replace binary_to_atom/1 with binary_to_existing_atom/1 in ssh_transport (valid_key_sha_alg_ec/2, public_algo/1). The input comes from ssh_message:oid2ssh_curvename/1 which returns a fixed set of 5 binaries whose atoms already exist as literals in the same module.
Replace list_to_atom/1 with an explicit pattern match in ssh_connection (pty_default_dimensions/2). The Dimension argument is always the hardcoded atom width or height.
Add a justification comment for the kept file:consult/1 directive in ssh_options — the file path is operator-controlled, not from wire data.